Repository navigation
hostmot2: fix PktUART queued counts and byte packing - #4659
sleepybishop wants to merge 8 commits into
Conversation
Return -EINVAL before computing the word count so negative requests cannot appear to succeed with zero reads. Test representative negative counts, including INT_MIN, and verify that they issue no LLIO calls.
| rtapi_u32 buff = ((rtapi_u32)data[c] | ||
| | ((rtapi_u32)data[c+1] << 8) | ||
| | ((rtapi_u32)data[c+2] << 16) | ||
| | ((rtapi_u32)data[c+3] << 24)); | ||
| r = hm2->llio->queue_write(hm2->llio, hm2->pktuart.instance[inst].tx_addr, &buff, sizeof(buff)); |
There was a problem hiding this comment.
Not sure I understand the reason to change this. The C standard states that calculation is performed on the native machine size. That would be at least 32-bit. Therefore the casts should be superfluous because the data array is already unsigned and its read is extended to a machine word before the shift operation.
(same comment longer down)
There was a problem hiding this comment.
Kept only the cast before << 24: unsigned char promotes to signed int, so the cast makes that shift unsigned. Restored the original additions.
There was a problem hiding this comment.
The conversion from "unsigned char" is to "unsigned int". The significance of signed/unsigned persists.
There was a problem hiding this comment.
Deferred in 65bfe2d. GCC 16/Clang 22 give 1 for _Generic(+(unsigned char)0, int: 1, default: 0), but both packing versions pass all 256 high-byte values with -fwrapv.
There was a problem hiding this comment.
unsigned uu(unsigned char c[]) {
return c[0] + (c[1] << 8) + (c[2] << 8) + (c[3] << 24);
}is on x86 translated by both gcc and clang into effectively one instruction (x86 allows unaligned access):
uu:
movl (%rdi), %eax
retqReversing the order of the bytes read (from 0123 to 3210) is also detected and it simply adds a bswapl instruction before the return. It is more work in non-natural orders.
Changing the signature to signed char changes the situation dramatically. You can change either unsigned into signed and see what the compilers do. On signed chars, the loads are movsbl, doing sign extension. If the shift load order is non-natural, then movzbl (zero extension) is used on the loads.
Split from #4488 as requested. Fixes PktUART queued-read counts and LLIO error handling. The count correction remains in its own commit; successful API returns are preserved.
Local builds pass. Mesa hardware validation remains pending.